-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(NSS): Fix should_pre_check logic to identify sshd #419
Conversation
The cmdline returned by calling proc.cmdline() has multiple strings that consist of the entire command, which means that, instead of having something like this: ["cmd", "arg1", "arg2, ..., "argn"] It looks like this: ["cmd arg1 arg2 ... argn"] In order to better limit the options, as this precheck can be a security breach, we only allow the precheck if it comes from the common known sshd binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, as discussed.
Mentioning it publicly too, for the record, I think we should also check for the current process PID (and not only on parent), to make this to work when nss is loaded directly by sshd (as it happens when you launch sshd not as a daemon), but it's likely just for debugging purposes for now.
However, note that will need it, when we will have an integration test that will launch sshd in a custom port and we'll try to login there...
So leaving it here as a remember us note for the future.
When the time comes for that, we can add the check for the current process locked under the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #419 +/- ##
==========================================
- Coverage 84.47% 84.47% -0.01%
==========================================
Files 77 77
Lines 6713 6711 -2
Branches 75 75
==========================================
- Hits 5671 5669 -2
Misses 730 730
Partials 312 312 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Phew! It wasn't a big conceptual issue afterall :)
The cmdline returned by calling proc.cmdline() has multiple strings that consist of the entire command, which means that, instead of having something like this: ["cmd", "arg1", "arg2, ..., "argn"] It looks like this: ["cmd arg1 arg2 ... argn"]
In order to better limit the options, as this precheck can be a security breach, we only allow the precheck if it comes from the common known sshd binary.
UDENG-3415